Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

CI for Focal and Jammy #10

Merged
merged 2 commits into from
Jun 3, 2022
Merged

CI for Focal and Jammy #10

merged 2 commits into from
Jun 3, 2022
+22 −0

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Jun 3, 2022

@chapulina this is what I had in mind for CI. I'm a bit weak on github actions so there may be syntax errors here. It uses some actions from https://github.com/ros-tooling/setup-ros

@sloretz
Copy link
Contributor Author

sloretz commented Jun 3, 2022 •
edited
Loading

I completly spaced that we can't really test sdformat_urdf on the ros2 branch builds on Focal because it would mean building all the ROS dependencies from source. My intent is this PR creates CI such that:

  • On galactic branch test SDFormat 9-12 using ROS Galactic
  • On ros2 branch test SDFormat 12 using ROS Humble and ROS Rolling

sloretz
sloretz commented Jun 3, 2022
View reviewed changes
Copy link
Contributor Author

@sloretz sloretz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - that is CI fails where I would expect it to (failing to find SDFormat9 on Jammy). Adding the GAZEBO_VERSION logic and support for sdformat12 should fix it.

chapulina
chapulina reviewed Jun 3, 2022
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I've never used actions from setup-ros because I usually need to inject custom logic, such as pulling packages from packages.osrfoundation.org, or skipping unknown rosdep keys. I wonder how it will handle the conditions on package.xml when it invokes rosdep. It will be great if it just works!

.github/workflows/focal-ci.yaml Outdated Show resolved Hide resolved
.github/workflows/focal-ci.yaml Outdated
@@ -0,0 +1,28 @@
name: focal-ci
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have you checked that this will work for the galactic branch even if it's merged into the ros2 branch?

In case it only works when it's backported to galactic, I'd argue that it's best not to add galactic CI information to the ros2 branch. Instead, you could give the yaml file a generic name, like ci.yaml, and modify it according to the branch. This also helps comparing it across branches, for example:

gazebosim/ros_gz@galactic...ros2#diff-7c3faec84b8df09639159898858a4155d55a7febe4d37a6700f26fb6bef9826e

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I updated the PR to have a generic name ci.yaml, but I don't have a specific commit with the change since I rebased and squashed to add the sign-off.

@chapulina chapulina mentioned this pull request Jun 3, 2022
Merged
@sloretz
CI for Focal and Jammy
4681b4c 
Signed-off-by: Shane Loretz <sloretz@openrobotics.org>
@sloretz
Only run on pull_request updates
4390712 
Signed-off-by: Shane Loretz <sloretz@openrobotics.org>
chapulina
chapulina approved these changes Jun 3, 2022
View reviewed changes
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for iterating. It looks like this should work, we'll know once the rosdep key and the source code changes from #9 are in.

@chapulina chapulina mentioned this pull request Jun 3, 2022
Closed
@sloretz sloretz merged commit 785d752 into ros2 Jun 3, 2022
@sloretz sloretz deleted the sloretz__ci_focal_jammy branch June 3, 2022 23:09
sloretz added a commit that referenced this pull request Jun 3, 2022
@sloretz
CI for Focal and Jammy (#10)
230eea3 
* CI for Focal and Jammy

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>

* Only run on pull_request updates

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>
sloretz added a commit that referenced this pull request Jun 8, 2022
@sloretz
[Galactic] Backport github actions CI (#11)
28565be 
* CI for Focal and Jammy (#10)

* CI for Focal and Jammy

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>

* Only run on pull_request updates

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>

* Focal;Galactic;Citadel,Edifice,Fortress

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>

* Gazebo -> gz

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>
sloretz added a commit that referenced this pull request Jun 8, 2022
@chapulina @sloretz
Galactic: Support Citadel, Edifice and Fortress (#8)
cc26b1f 
* CI for Focal and Jammy (#10)

* CI for Focal and Jammy

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>

* Only run on pull_request updates

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>

* Focal;Galactic;Citadel,Edifice,Fortress

Signed-off-by: Shane Loretz <sloretz@openrobotics.org>

* Galactic: Support Citadel, Edifice and Fortress

Signed-off-by: Louise Poubel <louise@openrobotics.org>

* Remove custom action

Signed-off-by: Louise Poubel <louise@openrobotics.org>

* cascading logic

Signed-off-by: Louise Poubel <louise@openrobotics.org>

* gazebo -> gz

Signed-off-by: Louise Poubel <louise@openrobotics.org>

* oops

Signed-off-by: Louise Poubel <louise@openrobotics.org>

* Reorder things

Signed-off-by: Louise Poubel <louise@openrobotics.org>

* Fix tests with Edifice

Signed-off-by: Louise Poubel <louise@openrobotics.org>

* no gz-utils for SDF 9

Signed-off-by: Louise Poubel <louise@openrobotics.org>

* uncrustify

Signed-off-by: Louise Poubel <louise@openrobotics.org>

* Apply suggestions from code review

Signed-off-by: Louise Poubel <louise@openrobotics.org>

Co-authored-by: Shane Loretz <shane.loretz@gmail.com>

* Citadel

Signed-off-by: Louise Poubel <louise@openrobotics.org>

Co-authored-by: Shane Loretz <sloretz@osrfoundation.org>
Co-authored-by: Shane Loretz <sloretz@openrobotics.org>
Co-authored-by: Shane Loretz <shane.loretz@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chapulina chapulina chapulina approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@sloretz @chapulina